Skip to content

fix(config): read float-declared keys with float defaults (#4925) - #4938

Merged
springfall2008 merged 1 commit into
mainfrom
fix/float-arg-truncation-4925
Oct 8, 2026
Merged

springfall2008 merged 1 commit into
mainfrom
fix/float-arg-truncation-4925

Conversation

@chalfontchubby

@chalfontchubby chalfontchubby commented Sep 5, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #4925.

Problem

get_arg() coerces its return value on the type of the default it is handed, and applies that to whatever value was resolved - real configured value or not. So get_arg("solcast_poll_hours", 8) ran a configured 4.8 through int(float(value)) and returned 4, shortening the Solcast poll TTL from 4.8h to 4h and pushing a two-site hobbyist account past its 10 poll/day quota into nightly HTTP 429s. Nothing warned: validate_config() checks the raw apps.yaml value against the float schema and passes it.

Fix

Float literals at every call site that reads an APPS_SCHEMA float-declared key with an int default - eight of them:

Where Key Default
components.py spec solcast_poll_hours 8 → 8.0
components.py spec forecast_solar_max_age 8 → 8.0
components.py spec open_meteo_forecast_max_age 4 → 4.0
components.py spec alphaess_api_delay 2 → 2.0
components.py spec axle_pence_per_kwh 100 → 100.0
octopus.py octopus_saving_session_rate (twice) 100 → 100.0, 0 → 0.0
octopus.py octopus_saving_session_min_octopoints_per_kwh 0 → 0.0

The issue's own suggestion (normalise in Components.initialize()) would miss the three direct octopus.py reads. A configured alphaess_api_delay of 0.5, for example, no longer becomes 0 and switches the AlphaESS throttle off.

This fixes the callers rather than get_arg(), as asked on review: the accessor stays keyed on its default, and a guard test stops the callers regressing instead.

User-visible, cosmetic only: a whole-number value now shows with ".0" - the Axle scheduled-event notification ("100.0 p/kWh"), the web UI's component config table, the Axle event attribute, Octopus saving-session octopoints_per_kwh when no reward is reported, and one Octopus log line.

Not included: the issue's second ask, a warning on lossy coercion. It would fire every cycle on keys that are integer by design, and the guard below prevents the float-key case at source.

Testing

  • test_float_declared_keys_never_read_with_int_default scans the source for any get_arg() call or COMPONENT_LIST spec giving a float-declared key an int literal default, and fails naming each. Against main it names all eight. It sees literals only - a default passed through a variable, or a wrapper accessor, is not checked.
  • test_component_arg_specs_resolve_float_declared_keys_as_float resolves each float-declared spec that has a default the way Components.initialize() does, and checks a configured 4.8 survives (5 specs). Fails against main's components.py.
  • ./run_all --quick and pre-commit run --all-files pass (exit 0).

Written by Claude on behalf of @chalfontchubby.

🤖 Generated with Claude Code

@springfall2008

Copy link
Copy Markdown
Owner

Not sure about this, shouldn't we fix the caller for get_arg()?

@chalfontchubby

Copy link
Copy Markdown
Collaborator Author

(Posted by Rik; written by Claude.)

Fair challenge, and normally I would agree — fix the caller, not the accessor. Two things pushed me the other way here.

"The caller" is not one place. Auditing main: 15 float-declared APPS_SCHEMA keys, affected in two unrelated mechanisms.

Where Keys
components.py arg specs solcast_poll_hours, forecast_solar_max_age, open_meteo_forecast_max_age
octopus.py direct get_arg() calls octopus_saving_session_rate (x2), octopus_saving_session_min_octopoints_per_kwh

The original issue proposed normalising in Components.initialize(), which fixes the first three and cannot reach the octopus.py ones — those never go through the component framework. So the caller-side fix is six edits across two mechanisms rather than one.

More to the point, it only holds while every future caller keeps getting it right. The trap resets every time someone writes 8 instead of 8.0 against a float-declared key, and it fails silently in the worst way: validate_config() reads the raw apps.yaml value against the float schema and passes it, so apps.yaml still says 4.8 while the runtime behaves as 4. Nothing warns, and the symptom surfaces somewhere unrelated — in #4925 as nightly Solcast HTTP 429s. A correctness property that depends on every call site remembering an invisible convention will drift back.

The framing that convinced me: the type is already declared authoritatively in APPS_SCHEMA, and get_arg() currently prefers an incidental literal over it. That reads as the accessor being wrong, not the callers.

There is also direct precedent — #4441 fixed this exact mechanism on the CONFIG_ITEMS route inside get_ha_config(), normalising an int default to float at the source for the same reason. This is the same bug on the apps.yaml/APPS_SCHEMA route, which get_ha_config()s normalisation cannot reach because there is no config_index entry to match. Fixing it in get_arg() keeps both routes consistent.

Two honest costs, so they are on the record rather than buried:

  • it adds an APPS_SCHEMA lookup to a hot accessor (dict .get on a resolved-once global, but it is not free); and
  • it makes get_arg()s return type depend on that global rather than purely on its arguments.

If you would rather keep the accessor dumb, the alternative I would want is all six call sites fixed plus a test that fails when a new int default appears against a float-declared key — the sweep in test_component_arg_specs_resolve_float_declared_keys_as_float already does roughly that for the component specs and could be widened. Without the guard I think it regresses quietly. Happy to switch it round if you prefer that shape — just say which.

@chalfontchubby

Copy link
Copy Markdown
Collaborator Author

(Posted by Rik; written by Claude.)

@springfall2008 following up — my reply above (2026-09-10) laid out the case for fixing this in get_arg() rather than at each caller, plus an alternative if you'd rather keep the accessor dumb (fix all six call sites + a regression test). Just flagging so it doesn't fall off your radar; happy to switch approach if you tell me which way you'd prefer.

@chalfontchubby
chalfontchubby force-pushed the fix/float-arg-truncation-4925 branch from 00e9266 to 95049fc Compare September 27, 2026 19:06
@chalfontchubby

Copy link
Copy Markdown
Collaborator Author

(Written by Claude, posted by @chalfontchubby.)

@springfall2008 I've switched this to your suggestion: the get_arg() change is gone, and the callers are fixed instead.

A correction to my earlier reply: it said six call sites, but there are eight - I had missed alphaess_api_delay and axle_pence_per_kwh in components.py. All eight now use float defaults (the table in the description lists them).

To stop a ninth appearing, there is a guard test that scans the source for any get_arg() call or COMPONENT_LIST spec giving a float-declared key an int literal default, and fails naming it. Against main it names all eight. The branch is rebased onto current main as one commit, and the description is rewritten to match.

@chalfontchubby chalfontchubby changed the title fix(config): trust APPS_SCHEMA float type over an int default in get_arg() (#4925) fix(config): read float-declared keys with float defaults (#4925) Oct 2, 2026
get_arg() coerces its return value on the type of the default it is
handed, and applies that to whatever value was resolved - real configured
value or not. So get_arg("solcast_poll_hours", 8) ran a configured 4.8
through int(float(value)) and returned 4, shortening the Solcast poll TTL
from 4.8h to 4h and pushing a two-site hobbyist account past its 10
poll/day quota into nightly HTTP 429s. Nothing warned: validate_config()
checks the raw apps.yaml value against the float schema and passes it.

Eight call sites read an APPS_SCHEMA float key with an int default: five
COMPONENT_LIST arg specs (solcast_poll_hours, forecast_solar_max_age,
open_meteo_forecast_max_age, alphaess_api_delay, axle_pence_per_kwh) and
three direct reads in octopus.py (octopus_saving_session_rate twice,
octopus_saving_session_min_octopoints_per_kwh). Each now has a float
literal. A configured alphaess_api_delay of 0.5, for example, no longer
becomes 0 and switches the AlphaESS throttle off.

Fixed at the callers rather than in get_arg(), as the maintainer asked on
review: the accessor stays keyed on the default, and a guard test stops
the callers regressing instead. It scans the source for any get_arg() call or COMPONENT_LIST spec that
gives a float-declared key an int literal default, and fails naming each
one (it lists all eight against main). A behavioural sweep also resolves
each float-declared COMPONENT_LIST spec that has a default the way
Components.initialize() does, and checks a configured fraction survives.

User-visible, cosmetic only: a whole-number value now shows with ".0" -
the Axle scheduled-event notification ("100.0 p/kWh"), the web UI's
component config table, the Axle event attribute, Octopus saving-session
octopoints_per_kwh when no reward is reported, and one Octopus log line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@springfall2008
springfall2008 force-pushed the fix/float-arg-truncation-4925 branch from 95049fc to 15cb16f Compare October 8, 2026 12:07
@springfall2008
springfall2008 self-requested a review October 8, 2026 12:07

@springfall2008 springfall2008 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. The callers now pass float defaults and get_arg() is unchanged, as asked for earlier. I checked every read of the 15 float-declared settings on current main: all are covered, and nothing downstream needs an integer. Both new tests passed on a local merge with main before the rebase.

One nit for a follow-up, not a blocker: test_component_arg_specs_resolve_float_declared_keys_as_float restores with my_predbat.args = original_args, which rebinds to a copy rather than restoring the shared dict in place.

(Review written by Claude on behalf of @springfall2008.)

@springfall2008
springfall2008 merged commit ff584d3 into main Oct 8, 2026
2 checks passed
@springfall2008
springfall2008 deleted the fix/float-arg-truncation-4925 branch October 8, 2026 12:27
springfall2008 added a commit that referenced this pull request Oct 9, 2026
…); mark GH#4925 fixed in PR #4938; note the PR #4738 time-entity escape hatch (#5453)

Folds (each verified against main c073c36 before landing):

- GH#5437 -> "Too many API errors" symptom row: one HA outage walks both
  mechanisms (REST counter + #5134's init failure); bare ValueError => empty
  str(e); fatal_error init-once, never cleared; full-log discriminator
  (Startup banners vs none); wrapper lives in predbat_addon (out of reach).
- GH#5438 -> Octopus row: vehicle-catalogue name matching keeps the last
  match; device attrs override configured rate/battery size outright
  (fetch.py "Take the max"); coordinator string-drop warning as marker.
- GH#5442 -> curve row: id == 0 caller gate in execute.py (both branches,
  lineage #683); find_charge_curve is per-inverter ("Inverter N Looking
  for" discriminator); one fleet-wide curve, not per-inverter suffixable;
  auto-mode re-run signature; debug-yaml double-args trap.
- GH#5444 -> Ohme row: no-active-session 404 class vs #4719 route
  withdrawal; unguarded control_charge() => 60s pause retry storm + health
  degrade; /config/ohme.py path trap; FINISHED excluded from
  CONNECTED_STATUSES gates the car plan (link marked unverified).
- GH#5445 -> new Excluded-load row: car_charging_energy is the generic
  excluded-load list; car_energy_reported_load is the inside-clamp flag
  (four consumer sites); no heat-pump key/diagram node; predheat row cross-ref.
- GH#5446 -> HA write/verify row: flag-vs-wiring contradiction (capability
  flag false silently discards apps.yaml wiring; user_configured_entity()
  protects only the four time entities); Wrote-X-successfully on a
  sensor.* placeholder is a self-echo, control_ledger ties log line to it.
- GH#5450 -> Savings row: restore/accumulate block, no plausibility guard,
  recorder-history fallback covers attribute reads; start_date = first
  #2844-run day, not a reset; recovery via state edit + re-stating
  start_date; #1114/#3108 archaeology ruled out as inflators.
- GH#5452 -> Compare row: standing charge excluded from compare
  (enable_standing_charge omits "compare" since #1952); compare config:
  blocks only override CONFIG_ITEMS; cost10 save-flip breaks plan HTML;
  Actual-vs-projection standing-charge gap; stale annual.py comments.

Re-check vs merged main:
- GH#4925 fixed in PR #4938 (merged 2026-10-08): entry updated, mechanism
  kept for older logs.
- PR #4738 (merged 2026-10-08) added user_configured_entity(): #5214 entry
  updated (time entities only) and GH#5446 extension added.
- Savings row's drifted line cites re-anchored on symbols.

Co-authored-by: CI <ci@example.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

solcast_poll_hours 4.8 is silently truncated to 4h, exhausting the Solcast daily quota

2 participants